Skip to content

Populate windowMinutes for MiMo and StepFun monthly windows (#2431) - #2526

Open
LeoLin990405 wants to merge 1 commit into
steipete:mainfrom
LeoLin990405:feat/mimo-stepfun-windowminutes
Open

Populate windowMinutes for MiMo and StepFun monthly windows (#2431)#2526
LeoLin990405 wants to merge 1 commit into
steipete:mainfrom
LeoLin990405:feat/mimo-stepfun-windowminutes

Conversation

@LeoLin990405

Copy link
Copy Markdown
Contributor

Summary

Companion to #2514 (#2431). MiMo's monthly token quota and StepFun's monthly credit pool build their primary RateWindow with windowMinutes = nil. planUtilizationSeriesSamples requires a window with windowMinutes, so those two cards get neither the plan-utilization history chart nor the predictive pace forecast that the Codex/Claude cards show — a real info-density gap.

Changes

  • MiMo (MiMoUsageSnapshot): the monthly token-quota window carries ProviderPaceCapability.monthlyWindowSentinelMinutes when planPeriodEnd is present (a real reset to project against), and nil otherwise.
  • StepFun (StepFunUsageFetcher): the credit-pool window carries it when creditResetTime is present (otherwise the pace projection would be meaningless).

Both are gated on a real reset date, so no window is fabricated for an account without one.

Verification

  • swift test --filter "MiMo|StepFun" — 115 passed, including new cases: StepFun's credit window now asserts windowMinutes == 43200, and MiMo asserts 43200 with a period end and nil without one.
  • swiftlint --strict — 0 violations; SwiftFormat — clean.
  • Live: a StepFun credit-plan account now reports the credit window with windowMinutes: 43200 (reset ≈ 1 month out) — it now feeds the history + pace forecast. (MiMo verified by construction; the account under test has no active plan period to exercise it live.)

Refs #2431.

…#2431)

MiMo's monthly token quota and StepFun's monthly credit pool built their
primary RateWindow with windowMinutes = nil, so `planUtilizationSeriesSamples`
skipped them — meaning those cards get neither the plan-utilization history
chart nor the predictive pace forecast that Codex/Claude cards show. This
brings them to parity, mirroring steipete#2514 (which did the same for Zai/Kimi/Grok/
Antigravity).

- MiMo: token window carries `monthlyWindowSentinelMinutes` when `planPeriodEnd`
  is present (a real reset to project against); nil otherwise.
- StepFun: the credit window carries it when `creditResetTime` is present.

Verified live: a StepFun credit-plan account now reports the credit window with
windowMinutes 43200 (reset ~1 month out). Tests cover both providers, including
the no-reset case that keeps windowMinutes nil.

Refs steipete#2431.
@clawsweeper clawsweeper Bot added the rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. label Jul 31, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c1928af5a7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

// Codex/Claude windows. Only when there's a real period end to reset against.
let windowMinutes = self.planPeriodEnd == nil
? nil
: ProviderPaceCapability.monthlyWindowSentinelMinutes

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Route monthly windows through history and card pace

Setting the sentinel here (and in the analogous StepFun change) still does not enable either advertised card feature: UsageStore.planUtilizationSeriesSamples routes generic providers only through exact 300/10080-minute session-equivalent windows, so a lone 43200-minute primary produces no history samples, while both provider descriptors retain the default .unsupported pace capability, causing resetWindowPaceDetail to return nil. Add explicit monthly sampling and .calendarMonthResetWindow capabilities for MiMo and StepFun, with a model-seam test that verifies the history and pace output rather than only the snapshot field.

AGENTS.md reference: AGENTS.md:L25-L27

Useful? React with 👍 / 👎.

Comment on lines +269 to +271
let creditWindowMinutes = self.creditResetTime == nil
? nil
: ProviderPaceCapability.monthlyWindowSentinelMinutes

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reject zero StepFun reset timestamps

When a credit-plan response contains "subscription_credit_reset_time": "0" (the API's existing representation for an unconfigured reset), parsing produces a non-nil Date(timeIntervalSince1970: 0), so this nil check incorrectly marks it as a real monthly window. Validate that the timestamp/reset date is positive and meaningful before assigning the monthly sentinel; otherwise downstream consumers receive a fabricated 30-day window anchored in 1970.

Useful? React with 👍 / 👎.

@LeoLin990405
LeoLin990405 force-pushed the feat/mimo-stepfun-windowminutes branch from c1928af to 117e855 Compare July 31, 2026 05:28
@clawsweeper clawsweeper Bot added status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. labels Jul 31, 2026
@clawsweeper

clawsweeper Bot commented Jul 31, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed July 31, 2026, 7:02 PM ET / 23:02 UTC.

ClawSweeper review

What this changes

The branch adds a monthly windowMinutes sentinel to MiMo token quotas and StepFun credit pools when a reset date is present, plus focused snapshot parsing tests.

Merge readiness

Blocked until stronger real behavior proof is added - 9 items remain

This PR is still necessary, but its central claim is not implemented: adding the 30-day sentinel alone does not route MiMo or StepFun into the generic history sampler or enable their card pace capability. The unchanged head also retains the zero-reset timestamp path, and the claimed live result does not include inspectable downstream history or pace output.

Priority: P2
Reviewed head: 117e85593cf335dbbb2cb4b7eafaf4a35368a206

Review scores

Measure Result What it means
Overall readiness 🧂 unranked krab (1/6) The patch has focused intent and passing reported checks, but it misses both downstream activation paths, retains invalid-reset handling, and lacks inspectable real behavior proof.
Proof confidence 🧂 unranked krab (1/6) Needs stronger real behavior proof before merge: The PR body reports a live StepFun windowMinutes value, but it does not provide inspectable after-fix output showing that the history chart and pace forecast actually appear; add redacted runtime output, logs, or a recording after the complete repair. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Patch quality 🦪 silver shellfish (2/6) 2 actionable review findings remain.

Verification

Check Result Evidence
Real behavior Needs proof Needs stronger real behavior proof before merge: The PR body reports a live StepFun windowMinutes value, but it does not provide inspectable after-fix output showing that the history chart and pace forecast actually appear; add redacted runtime output, logs, or a recording after the complete repair. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed 6 items Branch changes only snapshot metadata: The branch changes MiMo and StepFun primary windows from nil to the 30-day sentinel, but does not modify either provider descriptor, the history sampler, or the pace helper.
Generic sampling excludes a lone monthly primary window: For providers outside the explicit Codex, Claude, OpenCodeGo, and Antigravity branches, the sampler appends only resolved session and weekly components. MiMo and StepFun therefore do not record a lone 43,200-minute primary as a monthly history series.
Provider descriptors leave pace unsupported: Both descriptors omit the pace: initializer argument, while ProviderDescriptor defaults it to .unsupported; the menu helper returns no pace detail unless that capability accepts the window.
Findings 2 actionable findings [P2] Route monthly windows through history and card pace
[P2] Reject non-positive StepFun reset timestamps
Security None None.

How this fits together

MiMo and StepFun adapters turn provider API quota responses into RateWindow values. Those windows then feed CodexBar’s persisted utilization-history sampler and the menu card’s pace forecast, each of which has its own provider-specific eligibility rules.

flowchart TD
    A[Provider API quota response] --> B[MiMo and StepFun adapters]
    B --> C[Rate window metadata]
    C --> D[Usage history sampler]
    C --> E[Card pace eligibility]
    D --> F[Utilization history chart]
    E --> G[Pace forecast]
    F --> H[Menu card]
    G --> H
Loading

Before merge

  • Add real behavior proof - Needs stronger real behavior proof before merge: The PR body reports a live StepFun windowMinutes value, but it does not provide inspectable after-fix output showing that the history chart and pace forecast actually appear; add redacted runtime output, logs, or a recording after the complete repair. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
  • Route monthly windows through history and card pace (P2) - The new sentinel does not reach either advertised output. Generic providers append only session and weekly components in planUtilizationSeriesSamples, and both descriptors retain the default unsupported pace capability, so MiMo and StepFun still get no monthly history sample or resetWindowPaceDetail. Add explicit monthly sampling and .calendarMonthResetWindow descriptor capabilities with model-seam assertions. This remains from the prior review on unchanged code.
  • Reject non-positive StepFun reset timestamps (P2) - subscription_credit_reset_time: "0" parses as a non-nil epoch date, so this nil-only gate assigns a 30-day monthly sentinel to an unconfigured reset. Validate a positive, meaningful reset timestamp before creating a monthly window and cover the zero-value response.
  • Resolve merge risk (P1) - Merging as written would advertise monthly window metadata without producing the stated MiMo or StepFun utilization-history chart or pace forecast.
  • Resolve merge risk (P1) - A StepFun API reset value of 0 is currently converted into a non-nil epoch date, so enabling monthly pace without validating that timestamp can create a fabricated monthly window.
  • Complete next step (P2) - The source defects are concrete, but this external PR also requires contributor-supplied real behavior proof that an automated repair lane cannot establish on the contributor’s provider account.
  • Improve patch quality - Add explicit MiMo and StepFun monthly history sampling and calendar-month pace capabilities.
  • Improve patch quality - Reject zero or non-positive StepFun credit reset timestamps and add a focused parser regression test.
  • Improve patch quality - Post redacted after-fix terminal output, logs, or a recording that visibly demonstrates both the history and pace result; updating the PR body should trigger a new review, or a maintainer can comment @clawsweeper re-review.

Findings

  • [P2] Route monthly windows through history and card pace — Sources/CodexBarCore/Providers/MiMo/MiMoUsageSnapshot.swift:62-70
  • [P2] Reject non-positive StepFun reset timestamps — Sources/CodexBarCore/Providers/StepFun/StepFunUsageFetcher.swift:266-274
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Changed surface 4 files affected; 48 additions, 2 deletions The patch changes two adapter conversions and parser tests but no downstream history or menu-pace eligibility code.
Prior blocker status 2 P2 findings remain on an unchanged head The existing branch did not address the prior end-to-end behavior or reset-validity concerns.

Merge-risk options

Maintainer options:

  1. Complete the monthly-window path (recommended)
    Add the missing monthly sampler and pace capabilities, reject invalid StepFun reset timestamps, and prove the rendered model outputs before merging.
  2. Pause this narrow branch
    Close or pause the PR if maintainers do not want MiMo and StepFun monthly quotas to participate in history and forecast surfaces.

Technical review

Best possible solution:

Extend the provider contract end-to-end: explicitly sample valid monthly MiMo and StepFun windows, configure their descriptors for calendar-month reset pace, reject non-positive StepFun reset times, and add model-seam coverage proving both history and pace output.

Do we have a high-confidence way to reproduce the issue?

Yes, from current source: create a MiMo or StepFun snapshot with a 43,200-minute primary window and inspect the generic sampling and descriptor pace gates; both exclude the advertised downstream behavior.

Is this the best way to solve the issue?

No. Assigning the sentinel is necessary metadata but not sufficient; the narrow maintainable repair must also add explicit monthly history sampling, calendar-month pace capability, timestamp validation, and model-seam tests.

Full review comments:

  • [P2] Route monthly windows through history and card pace — Sources/CodexBarCore/Providers/MiMo/MiMoUsageSnapshot.swift:62-70
    The new sentinel does not reach either advertised output. Generic providers append only session and weekly components in planUtilizationSeriesSamples, and both descriptors retain the default unsupported pace capability, so MiMo and StepFun still get no monthly history sample or resetWindowPaceDetail. Add explicit monthly sampling and .calendarMonthResetWindow descriptor capabilities with model-seam assertions. This remains from the prior review on unchanged code.
    Confidence: 0.99
  • [P2] Reject non-positive StepFun reset timestamps — Sources/CodexBarCore/Providers/StepFun/StepFunUsageFetcher.swift:266-274
    subscription_credit_reset_time: "0" parses as a non-nil epoch date, so this nil-only gate assigns a 30-day monthly sentinel to an unconfigured reset. Validate a positive, meaningful reset timestamp before creating a monthly window and cover the zero-value response.
    Confidence: 0.98

Overall correctness: patch is incorrect
Overall confidence: 0.99

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 8ef86077e70a.

Labels

Label justifications:

  • P2: This is a bounded provider information-display defect: affected users lose forecast/history capability but core usage fetching remains available.
  • merge-risk: 🚨 other: The PR’s user-facing promise remains inactive after merge, and a malformed reset timestamp can be promoted into a monthly-window claim once the missing path is enabled.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦪 silver shellfish.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR body reports a live StepFun windowMinutes value, but it does not provide inspectable after-fix output showing that the history chart and pace forecast actually appear; add redacted runtime output, logs, or a recording after the complete repair. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

What I checked:

Likely related people:

  • steipete: Current-main blame attributes the history-support gate and both provider descriptor baselines to the v0.46.0 baseline commit; this is the central owner surface for deciding the intended monthly history and pace contract. (role: recent area contributor; confidence: high; commits: b1170d83963d, 8ef86077e70a; files: Sources/CodexBar/UsageStore+PlanUtilization.swift, Sources/CodexBarCore/Providers/MiMo/MiMoProviderDescriptor.swift, Sources/CodexBarCore/Providers/StepFun/StepFunProviderDescriptor.swift)
  • LeoLin990405: Recent StepFun credit-plan parsing work appears in the local file history and the current PR touches the same credit-window conversion path. (role: recent StepFun contributor; confidence: high; commits: 4a29d803bd33, 117e85593cf3; files: Sources/CodexBarCore/Providers/StepFun/StepFunUsageFetcher.swift, Tests/CodexBarTests/StepFunUsageFetcherTests.swift)

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (2 earlier review cycles)
  • reviewed 2026-07-31T05:31:32.904Z sha 117e855 :: needs real behavior proof before merge. :: [P2] Enable the downstream monthly history and pace paths | [P2] Reject non-positive StepFun reset timestamps
  • reviewed 2026-07-31T16:25:20.307Z sha 117e855 :: needs real behavior proof before merge. :: [P2] Route monthly windows through history and card pace | [P2] Reject non-positive StepFun reset timestamps

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Jul 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. P2 Normal priority bug or improvement with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant